Ignore exceptions thrown from Thread.UncaughtExceptionHandler #4525
Conversation
Fixes #4516 Background ---------- Whenever an *uncaught* exception happens in a coroutine, it gets reported to the `CoroutineExceptionHandler`. See <https://kotlinlang.org/docs/exception-handling.html#coroutineexceptionhandler>. However, if it's not installed, a platform-specific handler is used. On the JVM, this means invoking the thread's `UncaughtExceptionHandler`, which logs the exception to the console by default, but can be configured to do other things (for example, on Android, it will crash the application). Problem ------- User-specified `UncaughtExceptionHandler` instances are allowed to throw exceptions. Java's documentation says so (https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Thread.UncaughtExceptionHandler.html): > Any exception thrown by this method will be ignored by the Java Virtual Machine. This means a user is allowed to write a throwing `UncaughtExceptionHandler`, and the caller has to deal with it. In our implementation, however, we are simply invoking the exception handler as a plain function, and if that function throws an exception, we allow this exception to propagate to the coroutine machinery, causing it to fail. Solution -------- To comply with the contract defined for `UncaughtExceptionHandler`, we also ignore the exceptions thrown from there.
| * > Any exception thrown by this method will be ignored by the Java Virtual Machine. | ||
| * | ||
| * This means the authors of the thread exception handlers have the right to throw exceptions indiscriminately. | ||
| * We have no further channels for propagating the fatal exception, so we give up. */ |
There was a problem hiding this comment.
you can add something like val _ = "Welcome, weary traveler. To my breakpoint place" which is both self-explanatory and funny (okay, maybe only self-explanatory)
kotlinx-coroutines-core/jvm/src/internal/CoroutineExceptionHandlerImpl.kt
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt
Outdated
Show resolved
Hide resolved
kotlinx-coroutines-core/jvm/test/exceptions/CoroutineExceptionHandlerJvmTest.kt
Outdated
Show resolved
Hide resolved
| try { | ||
| exceptionHandler.uncaughtException(currentThread, exception) | ||
| } catch (_: Throwable) { | ||
| /* Do nothing. |
There was a problem hiding this comment.
Shall we also add a sample?
val handler = CoroutineExceptionHandler { _, e ->
// e is IllegalStateException("Will be handled")
throw IllegalStateException("Will be thrown and ignored")
}
GlobalScope.launch(handler) {
throw IllegalStateException("Will be handled")
}.join()
println("This sample finishes successfully, without any exceptions.")
There was a problem hiding this comment.
The test should serve as a sort of sample that is more precisely describing what happens and won't get outdated.
There was a problem hiding this comment.
Wouldn't we like to know if we change this behaviour?
There was a problem hiding this comment.
Yes, we would, which the test accommodates, but a textual sample does not.
There was a problem hiding this comment.
As in: the sample could be a test (and there's an effort to make it automatically a test)
There was a problem hiding this comment.
This is the behavior that testThrowingUncaughtExceptionHandler checks.
There was a problem hiding this comment.
I disagree but I exhausted the discussion budget on this. Let it be your way.
There was a problem hiding this comment.
Up to you, but if you're unclear on why testThrowingUncaughtExceptionHandler tests this exact behavior, please let me know.
| }) { | ||
| launch(Job()) { | ||
| expect(2) | ||
| throw TestException("to be reported") |
| expect(1) | ||
| val caughtException = catchingUncaughtException { | ||
| GlobalScope.launch(CoroutineName("last-ditch")) { | ||
| GlobalScope.launch(CoroutineName("last-resort")) { |
There was a problem hiding this comment.
Now I realised that CoroutineName("last-resort") should be just CoroutineName("some contextual information")
Fixes #4516
Background
Whenever an uncaught exception happens in a coroutine, it gets reported to the
CoroutineExceptionHandler. See https://kotlinlang.org/docs/exception-handling.html#coroutineexceptionhandler. However, if it's not installed, a platform-specific handler is used.On the JVM, this means invoking the thread's
UncaughtExceptionHandler, which logs the exception to the console by default, but can be configured to do other things (for example, on Android, it will crash the application).Problem
User-specified
UncaughtExceptionHandlerinstances are allowed to throw exceptions.Java's documentation says so
(https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Thread.UncaughtExceptionHandler.html):
This means a user is allowed to write a throwing
UncaughtExceptionHandler, and the caller has to deal with it.In our implementation, however, we are simply invoking the exception handler as a plain function, and if that function throws an exception, we allow this exception to propagate to the coroutine machinery, causing it to fail.
Solution
To comply with the contract defined for
UncaughtExceptionHandler, we also ignore the exceptions thrown from there.